Move azurite scaffolding #5448
Move azurite scaffolding #5448
Conversation
|
Fixed Issues (26)Great job! The following issues were fixed in this Pull Request
|
| @@ -66,6 +68,15 @@ public void ConfigureServices(IServiceCollection services) | |||
| services.Configure<IpRateLimitPolicies>(Configuration.GetSection("IpRateLimitPolicies")); | |||
| } | |||
|
|
|||
| // TODO: Be more selective about adding this scaffolder | |||
| if (Environment.IsDevelopment()) | |||
There was a problem hiding this comment.
We likely want to be much more strict on when this runs. Some additional checks I am considering:
- Have this be behind a
DEBUGcompiler directive - Only run this when ALL azure related connection strings are
UseDevelopmentStorage=true. - Only run this when ANY azure related connection strings are
UseDevelopmentStorage=true. - Run very selective scaffolding only for the things choosing
UseDevelopmentStorage=true. #1 and #2#1 and #3
| await ScaffoldAsync(cancellationToken); | ||
| } | ||
| // TODO: Handle certain errors with instructions on how to fix, like API version problems | ||
| catch (RequestFailedException requestedFailedEx) when (requestedFailedEx.ErrorCode == "InvalidHeaderValue") |
There was a problem hiding this comment.
Technically this error could be for a different header than the Api-Version header so I could also compare against the full exception message but I generally don't like that and have only ever seen this error because of that header.
| _logger = logger; | ||
| } | ||
|
|
||
| public async Task StartAsync(CancellationToken cancellationToken) |
There was a problem hiding this comment.
Any errors here, will stop the startup of the application. I could instead catch and not-rethrow errors to allow startup to continue but that will generally lead to the error showing up later in the application and likely not rethrowing a more descriptive error.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5448 +/- ##
==========================================
- Coverage 44.52% 44.43% -0.09%
==========================================
Files 1514 1516 +2
Lines 70403 70660 +257
Branches 6352 6367 +15
==========================================
+ Hits 31347 31398 +51
- Misses 37708 37910 +202
- Partials 1348 1352 +4 ☔ View full report in Codecov by Sentry. |
🎟️ Tracking
📔 Objective
I had A DAY with my development setup, most of the issue are unrelated to this PR but the last issue I had was an issue scaffolding Azurite. The issue was the current version of the powershell
Azmodule doesn't play nicely with theazurite:latest. To fix this I think a good course of action is lower the amount of versions in play from 3 to 2 and instead rely on the already referencedAzure.*packages. The versions of our packages don't always play nice with azurite either for example having to revert these upgrades: #5447.So instead of the developer having to manually run
setup_azurite.ps1during the setup process, it will happen automatically on the first start ofApi. Any issues with the setup will stop the startup ofApiand log the errors it had. This does run on every startup but doesn't clobber anything on subsequent runs.I have a few open questions that I made comments on the PR and welcome feedback on the approach. If we want to go with this approach and accompanying contributing-docs PR will be made.
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes